trcaz - Industry onboarding #986

trcazier · 2025-10-01T07:16:35Z

No description provided.

robodoo · 2025-10-01T07:16:39Z

[IMP] estate: add chapter 2 [IMP] estate: add chapter 3 [IMP] estate: add chapter 4 [IMP] estate: add chapter 5 [IMP] estate: add chapter 6 [IMP] estate: (chapter 7) add types, tags, & offers views/models and fix search group by & menu names [IMP] estate: add chapter 8 [IMP] estate: Chapter 10 [IMP] estate: add Chapter 11 [IMP] estate: apply some of Faruk's suggestions from the PR [IMP] estate: remove superfluous encoding line (runbot) [IMP] estate: add _ to UserError string parameters [IMP] estate: add chapter 12 [IMP] estate: fix the default date availability being computed only on upgrade [ADD] estate_account: add module manifest [IMP] estate: fix the default date lambda parameter error [IMP] estate_account: add chapter 13 [IMP] estate: add chapter 14 [IMP] estate, estate_account: fix author and license warning [IMP] estate: apply some PR suggestions [IMP] estate: prefix ondelete method with _unlink Co-authored-by: Cemal Faruk Güney <[email protected]> [IMP] estate: apply R&D string convention [IMP] estate: apply PR suggestions [IMP] estate_account: make action_property_sold more extensible and maintainable [IMP] estate, estate_account: implement tests and fix code to pass them [IMP] estate: implement test for garden uncheck behavior [IMP] estate: change test class names according to the conventions [IMP] estate: change references to self.properties[i] to their own variables for clarity [IMP] estate: change import style Co-authored-by: Cemal Faruk Güney <[email protected]> [IMP] estate: apply PR suggestions [IMP] estate: make test imports clean and not relative [IMP] estate_account: add test to check account.move creation on sell [IMP] estate_account: make test more robust by sorting invoice lines according to price [IMP] *: add empty tutorial commit

…n date

…e fields to property model and form view

…d in model and form

…d form

x_estate/models/x_real_estate_property_fields.xml

Co-authored-by: Abdelrahman Rashed <[email protected]>

abdrahmanrashed

great job, just some small things and you are good to go

abdrahmanrashed · 2025-10-03T14:34:22Z

x_estate/models/x_real_estate_property_fields.xml

+    <record id="api_public_write_rule" model="ir.rule">
+        <field name="name">Public users can't write</field>
+        <field name="model_id" ref="real_estate_property_model"/>
+        <field name="domain_force">[(0, '=', 1)]</field>


what is the purpose of this rule?
the domain is false for all instances right?

Yes, the tutorial said to allow read and write on the route (to allow the server action to be executed), but restrict the write user access with this domain

add an access right record to allow public users to read and write the model

prevent any write from the public user by adding a record rule for the write operation with an impossible domain (e.g. [('id', '=', False)])

x_estate/models/x_real_estate_property_fields.xml

x_estate/models/x_real_estate_property_model.xml

abdrahmanrashed · 2025-10-03T14:58:25Z

x_estate/views/x_real_estate_property_views.xml

+                <field name="x_bedrooms"/>
+                <field name="x_living_area"/>
+                <field name="x_expected_price"/>
+                <field name="x_date_availability" optional="disabled"/>


why is this set a disabled?

excellent question

Co-authored-by: Abdelrahman Rashed <[email protected]>

vava-odoo

Well done. Just a few comments reading through the xml part. Don't hesitate to let me know if anything is unclear

vava-odoo · 2025-10-07T20:26:58Z

x_estate/__manifest__.py

+{
+    'name': 'Estate (import)',
+    'depends': [
+        'base',


Every module depends on base, so base is only mentioned if there is no other.

Suggested change

'base',

vava-odoo · 2025-10-07T20:29:05Z

x_estate/models/x_real_estate_property_fields.xml

+        <field name="name">x_garden_orientation</field>
+        <field name="field_description">Garden Orientation</field>
+        <field name="ttype">selection</field>
+        <field name="selection">[('x_north', "North"), ('x_south', "South"), ('x_east', "East"), ('x_west', "West")]</field>


Wasn't it asked to use ir.model.fields.selection?

vava-odoo · 2025-10-07T20:29:58Z

x_estate/models/x_real_estate_property_fields.xml

+        <field name="model_id" ref="real_estate_property_model" />
+        <field name="name">x_total_area</field>
+        <field name="field_description">Total Area</field>
+        <field name="ttype">float</field>


Makes not much sense, but since other areas are integers, this one should be an integer as well

vava-odoo · 2025-10-07T21:11:24Z

x_estate/models/x_real_estate_property_fields.xml

+
+    <!-- Actions -->
+
+    <record id="api_public_write_rule" model="ir.rule">


If you have a security folder, this record should be there

vava-odoo · 2025-10-08T07:02:06Z

x_estate/models/x_real_estate_property_model.xml

+        <field name="model">x_estate.property</field>
+    </record>
+
+    <record model="ir.model.access" id="access_real_estate_property_public">


Same, should be in security/

vava-odoo · 2025-10-08T07:03:46Z

x_estate/models/x_real_estate_property_offer_fields.xml

+        <field name="selection">True</field>
+        <field name="selection">[('x_accepted', "Accepted"), ('x_refused', "Refused")]</field>


it makes no sense to define twice the same field (only last one is taken into account)

vava-odoo · 2025-10-08T07:04:23Z

x_estate/models/x_real_estate_property_offer_fields.xml

+        <field name="ttype">many2one</field>
+        <field name="relation">res.partner</field>
+        <field name="required">True</field>
+        <field name="on_delete">cascade</field>


a bit 🤠 but fine

vava-odoo · 2025-10-08T07:07:31Z

x_estate/views/x_real_estate_property_offer_views.xml

+        <field name="name">x_estate.property.offer.view.list</field>
+        <field name="model">x_estate.property.offer</field>
+        <field name="arch" type="xml">
+            <list string="Property offer list test string?">


vava-odoo · 2025-10-08T07:09:25Z

x_estate/views/x_real_estate_property_type_views.xml

+        <field name="name">Property types</field>
+        <field name="res_model">x_estate.property.type</field>
+        <field name="view_mode">list</field>
+        <field name="context">{'search_default_available': True}</field>


Does this work?

vava-odoo · 2025-10-08T07:09:51Z

x_estate/views/x_real_estate_property_tag_views.xml

+        <field name="name">Property tags</field>
+        <field name="res_model">x_estate.property.tag</field>
+        <field name="view_mode">list</field>
+        <field name="context">{'search_default_available': True}</field>


Does this work?

vava-odoo

Went on and read your whole code. Overall super good, just noted a few things.
No need to update anything here, just food for thought

vava-odoo · 2025-10-08T07:14:31Z

estate/data/estate.property.type.csv

@@ -0,0 +1,5 @@
+"id","name","sequence","property_ids","offer_ids","offer_count"


I bet you don't have to list all fields, only the one you define (and the required ones, obviously)

vava-odoo · 2025-10-08T07:15:05Z

estate/data/estate_property_data.xml

+    <record id="property_1" model="estate.property">
+        <field name="name">Big Villa</field>
+        <field name="property_type_id" ref="property_type_1"/>
+        <field name="state">new</field>


default value, not necessary

Suggested change

<field name="state">new</field>

vava-odoo · 2025-10-08T07:19:19Z

estate/models/estate_property.py

+    def _unlink_check_status(self):
+        for record in self:
+            if record.state not in ['new', 'cancelled']:
+                raise UserError(_("A property can only be deleted if its state is 'New' or 'Cancelled'"))


We try to have less technical messages for end-users, something like

Suggested change

raise UserError(_("A property can only be deleted if its state is 'New' or 'Cancelled'"))

raise UserError(_("A property can only be deleted if it is a new one or if it was cancelled."))

Also note that _() has been mostly replaced by self.env._()

vava-odoo · 2025-10-08T07:21:38Z

estate/models/estate_property.py

+            if not record.offer_ids:
+                raise UserError(_("A property with no offers cannot be sold."))


Isn't it too constraining the end user? You probably have specific cases where you want to bypass the "normal" flow and such conditions could be a burden for some users

vava-odoo · 2025-10-08T07:23:12Z

estate/models/estate_property_offer.py

+from odoo import _, api, exceptions, fields, models
+from odoo.exceptions import UserError


Suggested change

from odoo import _, api, exceptions, fields, models

from odoo.exceptions import UserError

from odoo import _, api, fields, models

from odoo.exceptions import UserError

vava-odoo · 2025-10-08T07:34:37Z

estate/views/estate_property_views.xml

+                <field name="bedrooms" />
+                <field name="living_area" filter_domain="[('living_area', '&gt;=', self)]"/>
+                <field name="facades" />
+                <filter name="available" string="Available" domain="['|', ('state', '=', 'new'), ('state', '=', 'offer_received')]"/>


shorter (and maybe clearer) this way

Suggested change

<filter name="available" string="Available" domain="['|', ('state', '=', 'new'), ('state', '=', 'offer_received')]"/>

<filter name="available" string="Available" domain="[('state', 'in', ['new', 'offer_received'])]"/>

vava-odoo · 2025-10-08T07:38:19Z

estate_account/models/estate_property.py

+        } for record in self]
+
+    def action_property_sold(self):
+        invoice_vals = self.get_sold_property_invoice_values()


nice way to do it

vava-odoo · 2025-10-08T07:38:48Z

estate_account/models/estate_property.py

+
+    def action_property_sold(self):
+        invoice_vals = self.get_sold_property_invoice_values()
+        self.env['account.move'].create(invoice_vals)


I fear you need a sudo here, as not all you sellers will be able to create an invoice

vava-odoo · 2025-10-08T07:40:17Z

estate_account/tests/test_estate_property.py

+        self.invoice = self.env['account.move'].search(Domain('partner_id', '=', self.cozy_cottage.buyer_id.id) & Domain('move_type', '=', 'out_invoice'))
+        self.assertTrue(self.invoice)


If you only need to check the existence, use search_count with limit=1 (if you need the first record, then only limit=1)

vava-odoo · 2025-10-08T07:41:38Z

estate_account/tests/test_estate_property.py

+        self.invoice = self.env['account.move'].search(Domain('partner_id', '=', self.cozy_cottage.buyer_id.id) & Domain('move_type', '=', 'out_invoice'))
+        self.assertTrue(self.invoice)
+
+        sorted_invoice_lines = self.invoice.invoice_line_ids.sorted(key=lambda r: r.price_unit)


What if the price of the house is really small? 🙃

trcazier requested a review from abdrahmanrashed October 1, 2025 07:16

trcazier force-pushed the 19.0-tutorial-industry-trcaz branch from 7ee5e1f to 4b5f71c Compare October 1, 2025 07:39

trcazier added 17 commits October 2, 2025 14:37

[IMP] estate: add CSV data file for property type

d31e6b3

[IMP] estate: add default property data

d1be6e1

[IMP] estate: add offer data & change property data ids

e065871

[IMP] estate: renamed property type data id for clarity

6b6d7c3

[FIX] estate: modify data to comply with estate model constraints

0c9993b

[FIX] estate: add xml file missing in manifest

b043d1a

[IMP] estate: improve 90% offer error message

c3bed90

[IMP] estate: add property type to property data

a127540

[FIX] estate: fix runbot warning for string

002dfba

[IMP] estate: modify date to be evaluated based on module installatio…

44f4e3e

…n date

[IMP] estate: accept 1 data offer and refuse 2 + fix same id

c0f4786

[IMP] estate: add new property with offers created inside the field

193323b

[FIX] estate: modify _ format parameters for runbot style

244c35d

[ADD] x_estate: add new importable module x_estate

a07d26c

[IMP] x_estate: add property_type model, views, menu, and add many2on…

1e82b73

…e fields to property model and form view

[IMP] x_estate: add property tags model, view, menu and property fiel…

bdb9f65

…d in model and form

trcazier force-pushed the 19.0-tutorial-industry-trcaz branch from f77cd91 to bdb9f65 Compare October 2, 2025 12:38

trcazier added 6 commits October 2, 2025 15:50

[CLN] x_estate: add empty line at the end of files

6def407

[IMP] x_estate: add offer model, view and add it to property model an…

0b05411

…d form

[FIX] x_estate: change offer buyer relation from user to partner

a5a4058

[IMP] x_estate: add selling price field to property model

8cb6ed6

[IMP] x_estate: add offer action to accept offer in list view

f368391

[IMP] x_estate: add total area computed field to property

94f4a3c

abdrahmanrashed reviewed Oct 3, 2025

View reviewed changes

x_estate/models/x_real_estate_property_fields.xml Outdated Show resolved Hide resolved

trcazier and others added 2 commits October 3, 2025 16:03

[IMP] x_estate: add json route to get properties as a public user

9bc6f35

[CLN] x_estate: reduce the number of lines

3e89bd5

Co-authored-by: Abdelrahman Rashed <[email protected]>

abdrahmanrashed reviewed Oct 3, 2025

View reviewed changes

trcazier and others added 4 commits October 3, 2025 17:04

[CLN] x_estate: reduce the number of lines

bad506f

Co-authored-by: Abdelrahman Rashed <[email protected]>

[CLN] x_estate: add empty line at the end of file

47d5056

[IMP] x_estate: remove optional=disabled from date availability

4348d79

[IMP] estate: make test data relative and remove optional

d9cc923

vava-odoo reviewed Oct 8, 2025

View reviewed changes


		<!-- Actions -->

		<record id="api_public_write_rule" model="ir.rule">

		<field name="selection">True</field>
		<field name="selection">[('x_accepted', "Accepted"), ('x_refused', "Refused")]</field>

		@@ -0,0 +1,5 @@
		"id","name","sequence","property_ids","offer_ids","offer_count"

	raise UserError(_("A property can only be deleted if its state is 'New' or 'Cancelled'"))
	raise UserError(_("A property can only be deleted if it is a new one or if it was cancelled."))

		if not record.offer_ids:
		raise UserError(_("A property with no offers cannot be sold."))

		from odoo import _, api, exceptions, fields, models
		from odoo.exceptions import UserError

	<filter name="available" string="Available" domain="['\|', ('state', '=', 'new'), ('state', '=', 'offer_received')]"/>
	<filter name="available" string="Available" domain="[('state', 'in', ['new', 'offer_received'])]"/>

		self.invoice = self.env['account.move'].search(Domain('partner_id', '=', self.cozy_cottage.buyer_id.id) & Domain('move_type', '=', 'out_invoice'))
		self.assertTrue(self.invoice)

trcaz - Industry onboarding #986

Are you sure you want to change the base?

trcaz - Industry onboarding #986

Uh oh!

Conversation

trcazier commented Oct 1, 2025

Uh oh!

robodoo commented Oct 1, 2025

Uh oh!

Uh oh!

abdrahmanrashed left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vava-odoo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vava-odoo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!